319004 - CVE-2005-4134 overlong document.title setting can corrupt history data, causing non-responsive temporary hang (crash?) on subsequent startups

Status: VERIFIED FIXED

(Core Graveyard :: History: Global, defect)

Description

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

From the URL:

  Basically firefox logs all kinda of URL data in it's history.dat file,
  this little script will set a really large topic and Firefox will then
  save that topic into it's history.dat.. The next time that firefox is
  opened, it will instantly crash due to a buffer overflow -- this will
  happen everytime until you manually delete the history.dat file -- which
  most users won't figure out.

We're probably blowing Mork's little mind, and it'll be easier to defend in the history code than in Mork.

People are reporting varying results in #developers.

Comment 1

[timeless]

i didn't keep the sample, but
232 morkBlob::GrowBlob(morkEnv*, nsIMdbHeap*, unsigned int)

was end of the line for the sample from osx (ff1.5rc2)

Comment 2

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

I can't reproduce the next-start hang or crash on OS X with 1.5, though I do get some beachballing when I run the script (15s or so).

Comment 3

[:Gavin Sharp [email: [e-mail]]]

Trying with a clean profile and a recent trunk build on Windows XP, I get a slight delay when clicking the link, and then an approximately 1.5 minute hang on the next startup. Everything seems to work fine after that, including the Go menu.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Stack from the hang on startup:

00 msvcrt!memcpy+0x33
01 mork!morkBlob::GrowBlob(class morkEnv * ev = 0x02850020, class nsIMdbHeap * ioHeap = 0x023e48e0, unsigned int inNewSize = 0xe7133)+0x7e [mozilla/db/mork/src/morkBlob.cpp @ 95]
02 mork!morkSpool::SpillPutc(class morkEnv * ev = 0x023e4900, int c = 0)+0x8d [mozilla/db/mork/src/morkSink.cpp @ 127]
03 mork!morkParser::ReadValue(class morkEnv * ev = 0x023e4930)+0xf8 [mozilla/db/mork/src/morkParser.cpp @ 937]
04 mork!morkParser::ReadCell(class morkEnv * ev = 0x0000003d)+0xd0 [mozilla/db/mork/src/morkParser.cpp @ 588]
05 mork!morkParser::ReadRow(class morkEnv * ev = 0x023e4900, int c = 91)+0xf0 [mozilla/db/mork/src/morkParser.cpp @ 677]
06 mork!morkParser::ReadTable(class morkEnv * ev = 0x023e4900)+0xb5 [mozilla/db/mork/src/morkParser.cpp @ 783]
07 mork!morkParser::ReadContent(class morkEnv * ev = 0x0252683a, unsigned char inInsideGroup = 0x00 '')+0x3b [mozilla/db/mork/src/morkParser.cpp @ 1432]
08 mork!morkParser::OnPortState(class morkEnv * ev = 0x0252683a)+0x24 [mozilla/db/mork/src/morkParser.cpp @ 1473]
09 mork!morkParser::ParseLoop(class morkEnv * ev = 0x0252683a)+0x59 [mozilla/db/mork/src/morkParser.cpp @ 1531]
0a mork!morkParser::ParseMore(class morkEnv * ev = 0x023e4900, int * outPos = 0x0012f568, unsigned char * outDone = 0x023e58a8 "", unsigned char * outBroken = 0x023e58a9 "")+0x40 [mozilla/db/mork/src/morkParser.cpp @ 1571]
0b mork!morkThumb::DoMore_OpenFileStore(class morkEnv * ev = 0x023e4900)+0x26 [mozilla/db/mork/src/morkThumb.cpp @ 479]
0c mork!morkThumb::DoMore(class morkEnv * ev = 0x023e4900, unsigned int * outTotal = 0x0012f5f8, unsigned int * outCurrent = 0x0012f5fc, unsigned char * outDone = 0x0012f61b "???", unsigned char * outBroken = 0x0012f61f "???")+0x47 [mozilla/db/mork/src/morkThumb.cpp @ 399]
0d mork!morkThumb::DoMore(class nsIMdbEnv * mev = 0x023e4928, unsigned int * outTotal = 0x0012f5f8, unsigned int * outCurrent = 0x0012f5fc, unsigned char * outDone = 0x0012f61b "???", unsigned char * outBroken = 0x0012f61f "???")+0x2e [mozilla/db/mork/src/morkThumb.cpp @ 194]
0e tkitcmps!NSGetModule+0xaa1a
0f tkitcmps!NSGetModule+0xa8ea
10 tkitcmps!NSGetModule+0x8607
11 docshell!nsDocShell::AddToGlobalHistory(class nsIURI * aURI = 0x023ca068, int aRedirect = 0, class nsIURI * aReferrer = 0x00000000)+0x30 [mozilla/docshell/base/nsDocShell.cpp @ 8092]
12 docshell!nsDocShell::OnNewURI(class nsIURI * aURI = 0x023ca068, class nsIChannel * aChannel = 0x023ca794, unsigned int aLoadType = 0, int aFireOnLocationChange = 0, int aAddToGlobalHistory = 37529492)+0x264 [mozilla/docshell/base/nsDocShell.cpp @ 7343]
13 docshell!nsDocShell::OnLoadingSite(class nsIChannel * aChannel = 0x023ca794, int aFireOnLocationChange = 0, int aAddToGlobalHistory = 1)+0x75 [mozilla/docshell/base/nsDocShell.cpp @ 7387]

Comment 5

[Simon Fraser [no longer active]]

I didn't get a crash in Camino (branch), but startup time was hugely increased.
In morkSpool::SpillPutc, mork is reading in a morkCoil, growing it 512 bytes at a time, and on each grow, copying the old buffer into the new buffer.

Comment 6

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Might be worth seeing if realloc there would help us, unless we want to just leave this for Fx2's storage-backed bookmarks/history/places/whatever.

Comment 7

[elfguy]

This is being reported as 'remote exploit' in the news:

http://digg.com/security/Kill_Firefox_1.5_with_remote_exploit

While it's not a security issue it should be fixed fast as malicious users may start using it now that it's known.

Comment 8

[Daniel Veditz [:dveditz]]

Dupe finder urls:
http://packetstormsecurity.org/0512-exploits/firefox-1.5-buffer-overflow.txt
http://www.securityfocus.com/brief/73

Comment 9

[Daniel Veditz [:dveditz]]

Attachment: testcase

Saving the testcase locally. This is the packetstorm version which adds some comments to the findhard.com version in the URL field.

Comment 10

[Rob]

This does not affect users that have configured Firefox to not save history data (Tools->Options->Privacy->History->Remember visited pages for the last [0] days).

Comment 11

[Juha-Matti Laurio]

What about using Sanitize feature, it will empty the broken History.dat when FF is closed. Setting 'Visited pages' to zero is not always a good decision. This Sanitizing would be a workaround?

Comment 12

[Juha-Matti Laurio]

It seems that the PoC was listed at http://www.milw0rm.com/dos.php 
(-> 2005-12-07) entitled as Firefox <= 1.5 (history.dat) Buffer Overflow Crash PoC

Direct link: http://www.milw0rm.com/id.php?id=1362

Comment 13

[Adam Guthrie]

*** Bug 319500 has been marked as a duplicate of this bug. ***

Comment 14

[Erik Fabert]

*** Bug 319542 has been marked as a duplicate of this bug. ***

Comment 15

[Daniel Schroeter]

Please change OS to "All" because the problem is also present in FF1.5 under Linux.
Everything gets very slow and FF will not be usable (also after restarts) until the history.dat file have been removed (or the wrong entry have been deleted).

Comment 16

[Marc]

A quick workarround is to set this setting:
user_pref("capability.policy.default.HTMLDocument.title.set","noAccess");

Comment 17

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

I wasn't able to reproduce the crash on startup with 1.5 under linux, OSX, or Windows; it's pretty slow on startup, but recovers (usually after about 15-30 seconds).

A simple fix would be to change history to only store a max of 512 characters of the document title; however, you can still do the same thing with the location (e.g. append #AAAAAAAAAAAAAA....), and we can't truncate that easily.

Comment 18

[Brendan Eich [:brendan]]

Can anyone reproduce a crash?  I can't.  I see the same stupid Mork code on the stack that Gavin reported in comment 4, and when that O(N^2) alg completes, the browser finishes starting up and seems in good health.

Who is claiming an exploitable crash?  Where is the stack backtrace or other evidence of exploitability?  The "PoC" doesn't prove its assertion, and all I see is a quadratic growth rate design botch in Mork, resulting in a fairly short non-responsive state (not a true hang) on startup.

We need more info to do something about this, unless it turns out not to be exploitable (in which case, the PoC author's name is mud in my book).

/be

Comment 19

[Brendan Eich [:brendan]]

Talkback IDs, anyone?  I can't find any by searching for milw0rm or findhard in the URL at either talkback-public or talkback-reports .m.o.

/be

Comment 20

[Brendan Eich [:brendan]]

(In reply to comment #17)
> A simple fix would be to change history to only store a max of 512 characters
> of the document title; however, you can still do the same thing with the
> location (e.g. append #AAAAAAAAAAAAAA....), and we can't truncate that easily.

We can clamp both URI and title at some generous but not-too-large upper bound (64K, e.g.), but we might want to fix the O(n^2) bug too.

/be

Comment 21

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

I've also tried this with windows with a 15000*10000 string (150MB or so); we baloon up to 300MB+ memory usage (UTF16), but no crash.  On restart, though, memory usage kept jumping between 68MB and 75MB, never really growing -- I didn't have a debug build handy, but it looks like it's just mork being really, really slow.  The same thing on linux; I have it up in the debugger now, and mork is happily getc()'ing and putc()'ing one character at a time.. it'll take it a while to get to 150MB, malloc'ing 512 more chars at a time and memcpy'ing the whole thing over.

Note that on windows I was a little worried that we may be overflowing some OS buffer when setting the window title; however, the WM_SETTEXT message takes a null-terminated string, so we're ok there.  This is just a really crappy perf bug that we ought to fix.

(IE seems to only store the original page title in history, before it's munged by script.)

Comment 22

[sendmail.to]

(In reply to comment #18)

[...]
> stack that Gavin reported in comment 4, and when that O(N^2) alg completes, the
[...]
> Who is claiming an exploitable crash?  Where is the stack backtrace or other
> evidence of exploitability?  The "PoC" doesn't prove its assertion, and all I
> see is a quadratic growth rate design botch in Mork, resulting in a fairly
> short non-responsive state (not a true hang) on startup.
[...]

Sounds a little bit like Bug317334. Is there a relation?

Comment 23

[Brendan Eich [:brendan]]

(In reply to comment #22)

> Sounds a little bit like Bug317334. Is there a relation?

No, that bug involves BiDi layout.  This is a history database O(n^2) bug.

/be

Comment 24

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: 319004-history-fix.patch (clamp URI/title to 64k)

This patch clamps the history service's URIs and page titles to 65k.

(Can someone who can wrap their head around our string API better than myself tell me how to accomplish what I want here without a string copy if the string is already less than 65k?)

Comment 25

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: 319004-mork-buffer-growth-fix.patch

Request the mork buffer to double in size while reading if it's below 64k, and increase by 64k after that.

Comment 26

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

The above two patches fix things for me; I get fairly quick startup even with super long strings in my history file (saved before the 64k patch).  I've got a third patch that has mork using realloc(), but it's more involved as it has to munge mork APIs to add Realloc() to its crazyness.  I'll attach it later, but we should probably not apply it.  The above two things should fix the problem.

Comment 27

[Mike Doty]

I can confirm the "DoS" with firefox-1.0.7 on a gentoo linux system.  Upon executing the javascript, both X and xfce4 immediatly hang, followed momentarly after with xfce4 then crashing.  Killing X is nessessary to restart the system.

After X and the WM have been restarted, firefox stars as usual.

Comment 28

[Matt Nordhoff (aka Peng on IRC & forums)]

(In reply to comment #27)
> I can confirm the "DoS" with firefox-1.0.7 on a gentoo linux system.  Upon
> executing the javascript, both X and xfce4 immediatly hang, followed momentarly
> after with xfce4 then crashing.  Killing X is nessessary to restart the system.
> 
> After X and the WM have been restarted, firefox stars as usual.

Maybe XFCE can't handle extremely long window titles? That would be an XFCE problem, not a Firefox problem. Though maybe Firefox should truncate them.

Comment 29

[Mike Doty]

just had some of my fellow devs test this and came up with the following results:

ff-1.5.0 + gnome(gtk): WM/X crash
ff-1.0.7 + xfce4(gtk): WM/X crash
ff-1.5.0 + kde(qt):    hang on startup until history.dat is removed

so no, I wouldn't call it a xfce bug, more likly a gtk bug.

Comment 30

[Matt Nordhoff (aka Peng on IRC & forums)]

Still, though, not a Firefox bug (In reply to comment #29)
> just had some of my fellow devs test this and came up with the following
> results:
> 
> ff-1.5.0 + gnome(gtk): WM/X crash
> ff-1.0.7 + xfce4(gtk): WM/X crash
> ff-1.5.0 + kde(qt):    hang on startup until history.dat is removed
> 
> so no, I wouldn't call it a xfce bug, more likly a gtk bug.

Still, though, not a Firefox bug, though if Firefox truncated really long titles the crashes would probably be prevented.

(Not completely bugspam, but I'll shut up now. :P )

Comment 31

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(In reply to comment #29)
> just had some of my fellow devs test this and came up with the following
> results:
> 
> ff-1.5.0 + gnome(gtk): WM/X crash

I see no crash with the X and gtk as shipped (and patched up) with Fedora Core 4.

Comment 32

[Robert Pena]

I am able to reproduce the crash on this machine:

AMD 2100+ w/ 512 MB PC2100, running Windows XP Pro SP2 and FF1.5. The test page was served by Apache 2.0.55, also on the same machine. 

I had to run the "exploit" several times before FF finally hosed. The first four times, I go to "Go" on the FF menu, and I would see "AAAAAAAAAAAAAAAAAAA....". I close FF and reopen, now "Go" shows "heh" instead. Run the "exploit" again, same results. FF runs noticeably slower after the third try. On the fifth, FF hangs with "Opening http://www.yahoo.com" in the status bar and a blank page. One Three-Finger-Salute later, and FF again crashes on try 6 with a "Looking up http://www.yahoo.com" message in the status bar. I tried FF about 5 or 6 more times with the same results. Fixed the problem by deleting "C:\Documents and Settings\<user>\Application Data\Mozilla\Firefox\Profiles\vm8tginr.default\history.dat". FF works flawlessly afterwards! 

(In reply to comment #18)
> Can anyone reproduce a crash?  I can't.  I see the same stupid Mork code on the
> stack that Gavin reported in comment 4, and when that O(N^2) alg completes, the
> browser finishes starting up and seems in good health.
> 
> Who is claiming an exploitable crash?  Where is the stack backtrace or other
> evidence of exploitability?  The "PoC" doesn't prove its assertion, and all I
> see is a quadratic growth rate design botch in Mork, resulting in a fairly
> short non-responsive state (not a true hang) on startup.
> 
> We need more info to do something about this, unless it turns out not to be
> exploitable (in which case, the PoC author's name is mud in my book).
> 
> /be
> 

Comment 33

[Simon Fraser [no longer active]]

Comment on attachment 205319 [details] [diff] [review]
319004-mork-buffer-growth-fix.patch

You should measure the bloat impact of this patch; mork might hold lots of small buffers around.

Comment 34

[Brendan Eich [:brendan]]

Comment on attachment 205318 [details] [diff] [review]
319004-history-fix.patch (clamp URI/title to 64k)

Can't you use Substring or nsDependentString?

/be

Comment 35

[Brendan Eich [:brendan]]

Comment on attachment 205319 [details] [diff] [review]
319004-mork-buffer-growth-fix.patch

Looks like we can't make the basis case for size (mBlob_Size) be a power of two, but the patch copes by testing > 65536 to switch from binary-exponential to linear growth.

/be

Comment 36

[Brendan Eich [:brendan]]

Comment on attachment 205319 [details] [diff] [review]
319004-mork-buffer-growth-fix.patch

smfr has mork experience, I can tell ;-).  Good point, this needs checking.  Trying for an sr from alecf, who knows mork and global hist too.

/be

Comment 37

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(In reply to comment #34)
> (From update of attachment 205318 [details] [diff] [review] [edit])
> Can't you use Substring or nsDependentString?

You would think, except I couldn't get the incantations to work right.  I'll try again tomorrow morning.

And yes, the bloat impact needs to be measured.  If it's significant, maybe we can have mork realloc "coils" down to their actual size once they're read in.

Comment 38

[Simon Fraser [no longer active]]

Comment on attachment 205319 [details] [diff] [review]
319004-mork-buffer-growth-fix.patch

I turned on MORK_DEBUG_HEAP_STATS in morkConfig.h and collected some data. This patch has almost no impact on mork's high water mark memory usage (~12.2Mb with my 9.1Mb history file).

Comment 39

[Juha-Matti Laurio]

An official statement including new workarounds for removing overly long Title items is located at 

http://www.mozilla.org/security/history-title.html

Comment 40

[Andrew Schultz]

Attachment: history fix with substring + xpfe

Comment 41

[Brendan Eich [:brendan]]

Comment on attachment 205381 [details] [diff] [review]
history fix with substring + xpfe

Looks good to me, vlad should r+.

String-fu sanity check from darin would be great too.  If there is a doc vlad missed, please point it out -- or if not, then I'm sure we can get this case doc'ed.  Thanks,

/be

Comment 42

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 205381 [details] [diff] [review]
history fix with substring + xpfe

>Index: toolkit/components/history/src/nsGlobalHistory.cpp
>-  const nsAFlatString& titleString = PromiseFlatString(aTitle);
>+  nsAutoString titleString(Substring(aTitle, 0, HISTORY_STRING_LENGTH_MAX));

This will always result in a copy, no?  I think we want to avoid given that the majority of strings passed in here will be less than HISTORY_STRING_LENGTH_MAX.

Comment 43

[[e-mail]]

Comment on attachment 205381 [details] [diff] [review]
history fix with substring + xpfe

> NS_IMETHODIMP
> nsGlobalHistory::SetPageTitle(nsIURI *aURI, const nsAString& aTitle)
...
>+  if (URISpec.Length() > HISTORY_STRING_LENGTH_MAX)
>+    URISpec.Left(URISpec, HISTORY_STRING_LENGTH_MAX);
We can't clamp the URI just here, we have to fix all the callsites if at all.

Comment 44

[Darin Fisher]

Comment on attachment 205381 [details] [diff] [review]
history fix with substring + xpfe

>Index: toolkit/components/history/src/nsGlobalHistory.cpp

>+// see bug #319004 -- clamp title and URL to generously-large but not too large
>+// length
>+#define HISTORY_STRING_LENGTH_MAX 65536

There is some intrinsic limitation of Mork that we are
concerned about here, right?  I think it would be good
to describe that intrinsic limitation here.

As for page titles, I think it would be find to limit
them by much more.  Maybe 1024 characters (or at most
4096) is enough title for any page?


> NS_IMETHODIMP
> nsGlobalHistory::SetPageTitle(nsIURI *aURI, const nsAString& aTitle)
> {
>   nsresult rv;
>   NS_ENSURE_ARG_POINTER(aURI);
> 
>-  const nsAFlatString& titleString = PromiseFlatString(aTitle);
>+  nsAutoString titleString(Substring(aTitle, 0, HISTORY_STRING_LENGTH_MAX));

You could probably use StringHead to simplify this slightly.

  nsAutoString titleString(StringHead(aTitle, HISTORY_STRING_LENGTH_MAX));

Ideally, the string code should be able to optimize away any
copying here if aTitle already contains a ref-counted buffer.
However, it does not do that today.  Probably worth filing a
bug to make it so.

You could write code like this if you really wanted to avoid
the copy:

  const PRUnichar *titleString;
                                                                                                                                                         nsAutoString titleBuf;
  if (aTitle.Length() > 1024 || !aTitle.IsTerminated()) {
    titleBuf = StringHead(aTitle, 1024);
    titleString = titleBuf.get();
  } else {
    titleString = aTitle.BeginReading();
  }

  
>+  if (URISpec.Length() > HISTORY_STRING_LENGTH_MAX)
>+    URISpec.Left(URISpec, HISTORY_STRING_LENGTH_MAX);

Hrm... I'm not sure that it is good to insert a modified URI into
global history.  Perhaps if the URI is that long, we should just
abort the history operation.

Comment 45

[Ben Fowler]

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20051208 Firefox/1.6a1

There is no crash and the browser behaves reasonably, in the sense of being
unresponsive for a few minutes and then recovering - consistent with the
other comments here.

Comment 46

[Brendan Eich [:brendan]]

(In reply to comment #44)
> (From update of attachment 205381 [details] [diff] [review] [edit])
> >Index: toolkit/components/history/src/nsGlobalHistory.cpp
> 
> >+// see bug #319004 -- clamp title and URL to generously-large but not too large
> >+// length
> >+#define HISTORY_STRING_LENGTH_MAX 65536
> 
> There is some intrinsic limitation of Mork that we are
> concerned about here, right?

Not with the other patch to this bug.  This is all just FYI, not to detract from your review comment cited above.

The underlying limitation in Mork was a bad O(n^2) growth rate (growing a buffer by 512 bytes at a time, indefinitely). But we want a sane upper bound on title and URL for history's own sake, even with the Mork flaw fixed.  With the Mork fix, Mork grows binary-exponentially till 64K and then switches to linear, so faces an O(n^2) cliff that's less severe.

So I suggested to vlad that he make the history code clamp at that threshold, so the only growth rate we ever see is O(log(n).  So (so!) you are right, there is a magic coupling here to comment! ;-)

/be

Comment 47

[Alec Flett]

Just happened across this from the sr request earlier.. but I have to agree with darin that actually using modifed URIs seems like a bad idea... the operation should just fail. 

At best it means that long links appear visited when they aren't.. at worst there's going to be some kind of exploit because of some other flaw in history or mork... 

i.e. someone is going to figure out how to make one URI look like another because of the truncation.

Comment 48

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Ok; how about this: we do a length check on the Title and the URI; if either exceeds 64k, we return NS_OK and silently ignore the history entry.  How's that sound?

Comment 49

[Matt Nordhoff (aka Peng on IRC & forums)]

(In reply to comment #48)
> Ok; how about this: we do a length check on the Title and the URI; if either
> exceeds 64k, we return NS_OK and silently ignore the history entry.  How's that
> sound?

I'm not a developer so my opinion isn't greatly important, but I think it should be safe enough to just snip long titles but still store them. Snipping URIs sounds like a bad idea because the shorter version might actually be a different valid page, but the titles aren't important like that.

So, do the NS_OK and silently not adding long URIs, but just chop down long titles.

Comment 50

[Brendan Eich [:brendan]]

Any URI 64K or longer is a fake one generated by a nasty script, I say (anyone with a counterexample from a real server, please cite it).  So we could bail on overlong URI too, and never break any real case that matters.

Some kind of message dumped to the JS console might be appropriate, to let savvy users and debuggers-in-the-field know what happened, but NS_OK is the only way to be sure we won't propagate a failure up into the user's face, or into a latent bug.

/be

Comment 51

[georgi - hopefully not receiving bugspam]

couldn't reproduce this, but on the full disclosure list a *blue screen* is claimed by a modified poc.

http://lists.grok.org.uk/pipermail/full-disclosure/2005-December/039784.html
--quote--
It works here.

seems it depends on how much ram you've. i got 2 blue screens, after
changed the code a bit. the first one was about MEMORY_MANAGEMENT and
the second one was a PAGE_FAULT_IN_NONPAGED_AREA. And both occurs
without user interaction, the second one i just've opened firefox, not
the bug file (maybe cache ?)

ps: i've 1Gb of ram
--quote--

Comment 52

[georgi - hopefully not receiving bugspam]

(In reply to comment #50)
> Any URI 64K or longer is a fake one generated by a nasty script, I say (anyone
> with a counterexample from a real server, please cite it).  So we could bail on

at least the canvas element may generate long data URIs:
http://www.whatwg.org/specs/web-apps/current-work/#htmlcanvaselement
interface HTMLCanvasElement : HTMLElement {
....
// returns a data: URI representing the current image as a PNG
  DOMString toDataURL();

Comment 53

[Kevin Brosnan]

*** Bug 319806 has been marked as a duplicate of this bug. ***

Comment 54

[Darin Fisher]

> at least the canvas element may generate long data URIs:

The history system refuses to store data: URIs:
http://lxr.mozilla.org/mozilla/source/toolkit/components/history/src/nsGlobalHistory.cpp#581

Odd that it does not include javascript URIs since those could be very long as well if they are being used to pass around data.

Comment 55

[Darin Fisher]

Andrew: Are you going to post a revised patch?

Comment 56

[Andrew Schultz]

Attachment: updated patch

this clamps the title at 4096 and drops URIs > 64K at all the public call sites (or wherever they end up)

Comment 57

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 205799 [details] [diff] [review]
updated patch

I'm fine with this, even though it always copies the string.. trying to make it not do that seems to be a black art.

I think blake (mrbkap) was doing another patch, that fixed the actual calls to set the document title to clamp to 4k.  We should probably take that one as well.

Comment 58

[Simon Fraser [no longer active]]

Could you include Camino's nsSimpleGlobalHistory in the patch too? Thanks.

Comment 59

[Andrew Schultz]

Attachment: xpfe patch applied to camino (untested)

Comment 60

[Andrew Schultz]

*** Bug 92062 has been marked as a duplicate of this bug. ***

Comment 61

[Christian :Biesinger (don't email me, ping me on IRC)]

+  nsAutoString titleString(StringHead(aTitle, HISTORY_TITLE_LENGTH_MAX));

this might split a character in the middle :-/ (a non-BMP one)

Comment 62

[Tom Hessman]

(In reply to comment #16)
> A quick workarround is to set this setting:
> user_pref("capability.policy.default.HTMLDocument.title.set","noAccess");

It should be noted that setting that pref only works around a JavaScript version of this PoC.  It was incredibly easy to modify it to use server-side scripting such as PHP and see the same effects, and even to generate a plain HTML file that does the trick.  Many people seem to think that using the NoScript extension or otherwise blocking JavaScript protects them, which simply isn't true (as I assume most of you know).

Also, by increasing the number of iterations of the first loop to 10000 and the second to 5000, my Mac started to completely lock up and become unusable.  After a few minutes, I got a warning that I was low on disk space, so I reset my machine.  While I don't remember for sure what my free disk space was before, I don't have much less now, and there seems to be about a 2.5GB discrepency between what I can identify as taking up space and how much space is being used.  My first thought was some sort of cache file in my Firefox profile, but that doesn't seem to be the case; my profile is only about 357.6MB (mostly due to bug 157152 I assume; I'm up to bookmarks-1283.html now)...

Comment 63

[Giorgio Maone [:ma1]]

(In reply to comment #62)
> Many people seem to think that using the
> NoScript extension or otherwise blocking JavaScript protects them, which simply
> isn't true (as I assume most of you know).

Using NoScript version 1.1.3.5 *is* actually effective also against server-side exploits, since it truncates document titles to 255 characters by default.
In facts, I released 1.1.3.5 as soon as I became aware of this bug and of the general belief you're referring to (Dec 7, IIRC).

Of course you're right about blocking JavaScript otherwise (e.g. through the Firefox Content Options). 

Comment 64

[Andrew Schultz]

Comment on attachment 205799 [details] [diff] [review]
updated patch

This is in the trunk.

This should probably go on the branches too.  Risk should be low.

I filed bug 322167 for comment 61

Comment 65

[Daniel Veditz [:dveditz]]

If this is fixed on trunk can it be resolved FIXED then? Are we missing a piece before we can consider it for the branches?

Comment 66

[Andrew Schultz]

We're missing a fix for bug 322167, although that doesn't seem very serious.  The number of real titles > 4096 characters has to be exceptionally small and the worst that would happen is to have a dangling byte at the end of the title (don't know how serious that is).  I hope to get a fix for that in the next day or so.

Comment 67

[Christopher Aillon (sabbatical, not receiving bugmail)]

What is the plan for attachment 205319 [details] [diff] [review]?  That has r+sr and seems to be a good fix based on comment 26 and comment 38.

Comment 68

[Daniel Veditz [:dveditz]]

Comment on attachment 205799 [details] [diff] [review]
updated patch

a=dveditz for 1.8/1.8.0.1 branches. Please add the fixed1.8.1 and fixed1.8.0.1 keywords when checked in.

Comment 69

[Andrew Schultz]

attachment 205799 [details] [diff] [review] is in both 1.8 branches

Comment 70

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 205319 [details] [diff] [review]
319004-mork-buffer-growth-fix.patch

Still haven't seen a checkin for this patch yet (or heard a good reason why it should not be comitted.)

Comment 71

[Jay Patel [:jay]]

v.fixed on 1.8.0.1 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.1) Gecko/20060109 Firefox/1.5.0.1, no hang/crash on restart using buffer overflow testcase.

Comment 72

[Simon Fraser [no longer active]]

Comment on attachment 205932 [details] [diff] [review]
xpfe patch applied to camino (untested)

I checked this patch into trunk, 1.8 and 1.8.0 branches for Camino.

Comment 73

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(In reply to comment #70)
> (From update of attachment 205319 [details] [diff] [review] [edit])
> Still haven't seen a checkin for this patch yet (or heard a good reason why it
> should not be comitted.)

I believe we decided not to touch mork at all, and just fix the caller -- the bad growth handling is just a slowdown, and not a big deal with a 65k cap.  Noone seemed to be sure how fragile mork was, no matter how safe the patch looked.

Comment 74

[David :Bienvenu]

Re the mork patch, I doubt it's going to break anything, but it might increase Mork's memory usage substantially. I'd need to check what kind of data this code is used on, and the lifetime of the storage it allocates. If this memory stays allocated while the db is open, I'd suggest only doing the doubling of the size if the string is > 2K or so, since strings like that are going to be relatively rare.

Comment 75

[Daniel Veditz [:dveditz]]

*** Bug 324020 has been marked as a duplicate of this bug. ***

Comment 76

[Simon Fraser [no longer active]]

(In reply to comment #74)
> Re the mork patch, I doubt it's going to break anything, but it might increase
> Mork's memory usage substantially.

See comment #38 :) I only measure global history though, and not other mork clients.

Comment 77

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: Port to Firefox 1.0.7

Just for reference, 3 out of the hunks failed when applying to aviary 1.0.x, so this is an patch which applies to aviary.  The patch looks almost identical to what was checked in.  The patch failed to apply mostly because of surrounding code changing.

Comment 78

[Daniel Veditz [:dveditz]]

Comment on attachment 210094 [details] [diff] [review]
Port to Firefox 1.0.7

a=dveditz for aviary101/moz17 branches

Comment 79

[Daniel Veditz [:dveditz]]

checked in to aviary101/moz17 branches

Comment 80

[Phil Ringnalda (:philor)]

*** Bug 325414 has been marked as a duplicate of this bug. ***

Comment 81

[Rich Painter]

Advisory 2006-03 http://www.mozilla.org/security/announce/mfsa2006-03.html indicates that this flaw is also in Mozilla but no references seem to be here about fixing this.  This Advisory does indicate to set the History Duration to 0 days.

Is someone fixing this for Mozilla Suite and when will it be available?

thanks,
r

Comment 82

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #81)
> Is someone fixing this for Mozilla Suite and when will it be available?

It's already fixed for the Mozilla Suite. The keyword "fixed1.7.13" is telling it. You have to wait for the next bugfix release 1.7.13 of Mozilla.

Comment 83

[Tracy Walker [:tracy]]

verified with:
Windows:
Moz - Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.13) Gecko/20060214
Fx - Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.13) Gecko/20060214
Firefox/1.0.8
Macintosh:
Moz - Mozilla/5.0 (Macintosh; U;PPC Mac OS X Mach-O; en-US; rv:1.7.13)
Gecko/20060214 Firefox/1.0.8
Fx - Mozilla/5.0 (Macintosh; U;PPC Mac OS X Mach-O; en-US; rv:1.7.13)
Gecko/20060214 Firefox/1.0.8
Linux
Moz - Mozilla/5.0 (X11; U;Linux i686; en-US; rv:1.7.13) Gecko/20060214
Fx -  Mozilla/5.0 (X11; U;Linux i686; en-US; rv:1.7.13) Gecko/20060214
Firefox/1.0.8